-
Notifications
You must be signed in to change notification settings - Fork 513
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Stable TableRow converted from BQ types #5536
Conversation
val provider: OverrideTypeProvider = | ||
OverrideTypeProviderFinder.getProvider | ||
val s = q"$tree.toString" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we were forcing toString
before converting back to desired type
fdd4fbb
to
d8d30c8
Compare
// f is a field from TableRow. | ||
// Jackson ObjectMapper will fail with such key | ||
key <- Gen.alphaStr.retryUntil(_ != "f") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering what happens if a BQ table has a field named f
. It's probable that we can't use the TableRow
API
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did a quick test. Reading a column named 'f' fails to create a TableRow
item here.
d8d30c8
to
d76d226
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5536 +/- ##
==========================================
+ Coverage 61.44% 61.85% +0.41%
==========================================
Files 312 312
Lines 11105 11207 +102
Branches 776 791 +15
==========================================
+ Hits 6823 6932 +109
+ Misses 4282 4275 -7 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally all the TableRow -> scala
cast logic should be shared with TableRowOps
a820b88
to
f92e641
Compare
Coder[TableRow] is destructive (it is a dummy JSON serializer), we should make sure that the TableRow object converted from a BQ model is stable after serialization. We currently have an issue with - long that are serialized as string to avoid overflow - float that are read back as double - json that is read as nested TableRow
7694e15
to
c869876
Compare
row.getRecord("record") shouldBe expected | ||
} | ||
|
||
it should "#3378: not throw an NPE on a non-existent subrecord" in { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not very logical.
Added a getRecordOpt
to be able to get Option[TableRow]
instead of returning null
private lazy val mapper = new ObjectMapper() | ||
.registerModule(new JavaTimeModule()) | ||
.registerModule(new JodaModule()) | ||
.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what's this needed for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the config from TableRowJsonCoder.
It forces serialization of dates as string
Feature that determines whether Date (and date/ time) values (and Date-based things like java. util. Calendars) are to be serialized as numeric time stamps (true; the default), or as something else (usually textual representation).
Coder[TableRow] is destructive (it is a dummy JSON serializer), we should make sure that the TableRow object converted from a BQ model is stable after serialization.
We currently have an issue with
As side effect, I needed to avoid
toString
conversion when converting back a BQ typed model from aTableRow